Skip to content

Add resolveNamespaceID - #263

Open
JayChung0258 wants to merge 1 commit into
mainfrom
jayhung/sa-2-owner-table
Open

Add resolveNamespaceID#263
JayChung0258 wants to merge 1 commit into
mainfrom
jayhung/sa-2-owner-table

Conversation

@JayChung0258

Copy link
Copy Markdown
Contributor

The main idea

To translate a search attribute per namespace, the code first has to answer a question it cannot answer today: standing on a SearchAttributes field somewhere in a replication message, which namespace does this belong to?

The attribute itself does not say. It is a map key with an opaque payload under it. The namespace is further out, on the message that owns the workflow.

This PR adds the one function that answers that. Nothing calls it yet.

What was changed

resolveNamespaceID(vwp, fallback) walks up the parent chain from any field and returns the NamespaceId of the nearest message that owns a namespace:

SyncVersionedTransitionTaskAttributes
  NamespaceId: ns-a                    <- the answer, two hops up
  VersionedTransitionArtifact          <- no NamespaceId of its own
    EventBatches                       <- start here

fallback is returned when the walk reaches nothing. That happens in two places, both handled in later PRs: inside a data blob, whose events are visited in a fresh traversal with no link back out, and on the two raw history responses, which carry history but no namespace of their own.

Two decisions worth reviewing

Owners are matched by a type switch over four types, not by looking for a field named NamespaceId.

switch owner := p.Addr().Interface().(type) {
case *replicationspb.HistoryTaskAttributes:
case *replicationspb.BackfillHistoryTaskAttributes:
case *replicationspb.SyncVersionedTransitionTaskAttributes:
case *persistencespb.WorkflowExecutionInfo:
}

Because plenty of other messages have a NamespaceId that means something else. history.StartChildWorkflowExecutionInitiatedEventAttributes is the one that would actually bite: it holds the child's NamespaceId (field 18) right next to the parent's SearchAttributes (field 17), so a field name match would translate a parent's history event with the child's mapping. WorkflowExecutionInfo has the same shape via ParentNamespaceId.

A type switch also means the compiler checks this. If one of these messages renames its field upstream, the build breaks instead of translation quietly stopping.

The question I would like a second opinion on: are these four complete? I got them by going through every search attribute and data blob field reachable in adminservice, replication, persistence and history. If a new message type shows up later with search attributes and an owner that is not in this list, resolution returns the fallback and translation is skipped rather than being wrong. A later PR adds a counter for that case.

The walk goes up, not down. Remembering the last NamespaceId seen while descending would be simpler to write, but the visit library does not define an order: visit.ValuesUnsafe pops the front of its worklist and swaps in the last element, so it is neither breadth first nor depth first. One replication frame can carry tasks for several namespaces, so state kept during a descent can be latched from the wrong task. Walking up from the field has no such problem.

Checklist

  1. Toward CGSCE-639.

  2. How was this tested:

Six tests, all walking real messages so they exercise the same parent chain the translator will see: one hop, two and three hops, the child NamespaceId being stepped over, ParentNamespaceId being ignored, an owner with an empty id carrying on outward, and no owner returning the fallback.

Mutation checked: adding StartChildWorkflowExecutionInitiatedEventAttributes to the type switch turns TestResolveNamespaceIDIgnoresChildNamespaceID red, which is that test's only job.

make generate-test-certs
go test -race -timeout=12m -tags test_dep -count=1 ./...
make lint

Both clean.

  1. Any docs updates needed?

No.

🤖 Generated with Claude Code

Given a field somewhere in a replication message, resolveNamespaceID walks up
the parent chain and returns the namespace id of the nearest message that owns
a namespace. Nothing calls it yet.

Only four message types count as owners, matched by a type switch. Other
messages carry a NamespaceId that names a different namespace, so matching on
the field name would pick the wrong one. The clearest example is
StartChildWorkflowExecutionInitiatedEventAttributes, which holds the child's
NamespaceId right beside the parent's SearchAttributes.

Walking up rather than remembering the last NamespaceId seen on the way down
keeps the answer independent of traversal order, which the visit library does
not define.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@JayChung0258
JayChung0258 requested a review from a team as a code owner August 25, 2026 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant